Fix: Enable save button for provider dropdown and checkbox changes #7113
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The save button wasn't enabling when users changed provider settings like dropdowns and checkboxes because setApiConfigurationField was treating all changes from undefined to a defined value as 'initial sync' and not marking the form as dirty. Added an optional isUserAction parameter (defaults to true) to distinguish: - User actions (should enable save button) - the default - Automatic initialization (shouldn't enable save button) - pass false This fixes the issue where changing provider dropdowns, checkboxes with default values, and other settings wouldn't enable the save button.
dosubot
bot
added
size:M
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Thank you for your contribution! This PR successfully fixes the save button enablement issue for provider settings. The solution is clean, backward-compatible, and well-implemented. I've left some suggestions inline to help improve the implementation.
| }) | ||
| }, []) | ||
|
|
||
| const setApiConfigurationField = useCallback( |
Contributor
There was a problem hiding this comment.
Consider adding JSDoc documentation for this function to explain the purpose of the isUserAction parameter. This would help future developers understand when to pass false. For example:
Suggested change
| const setApiConfigurationField = useCallback( | |
| /** | |
| * Updates a field in the API configuration | |
| * @param field - The field to update | |
| * @param value - The new value | |
| * @param isUserAction - Whether this change is from user interaction (true) or automatic initialization (false). | |
| * User actions will mark the form as dirty and enable the save button. | |
| */ | |
| const setApiConfigurationField = useCallback( | |
| <K extends keyof ProviderSettings>(field: K, value: ProviderSettings[K], isUserAction: boolean = true) => { |
| const isInitialSync = previousValue === undefined && value !== undefined | ||
| // Only skip change detection for automatic initialization (not user actions) | ||
| // This prevents the dirty state when the component initializes and auto-syncs values | ||
| const isInitialSync = !isUserAction && previousValue === undefined && value !== undefined |
Contributor
There was a problem hiding this comment.
The logic here is correct, but consider adding a comment to explain why we skip change detection for automatic initialization. This would help future maintainers understand the pattern.
| if (!selectedModelId && !isInitialized.current) { | ||
| const initialValue = modelIds.includes(selectedModelId) ? selectedModelId : defaultModelId | ||
| setApiConfigurationField(modelIdKey, initialValue) | ||
| setApiConfigurationField(modelIdKey, initialValue, false) // false = automatic initialization |
Contributor
There was a problem hiding this comment.
Good use of the comment here to clarify that this is automatic initialization! This pattern makes the intent very clear.
The useEffect that syncs apiModelId with selectedModelId was incorrectly passing isUserAction=false, which prevented the save button from enabling when users selected a different model. Since this effect responds to all selectedModelId changes (including user selections), it should use the default isUserAction=true behavior.
hannesrudolph
added
the
Issue/PR - Triage
The test now correctly expects setApiConfigurationField to be called with three arguments including the isUserAction=false parameter for automatic thinking token adjustments.
hannesrudolph
added
PR - Needs Review
and removed
Issue/PR - Triage
mrubens
approved these changes
Aug 14, 2025
github-project-automation
bot
moved this from PR [Needs Review]
to Done
in Roo Code Roadmap
This was referenced Aug 16, 2025
fxcl
added a commit
to Anabaai/Roo-Cline
that referenced
this pull request
Aug 16, 2025
* main: (70 commits) fix: use native Ollama API instead of OpenAI compatibility layer (RooCodeInc#7137) feat: add support for OpenAI gpt-5-chat-latest model (RooCodeInc#7058) Make enhance with task history default to true (RooCodeInc#7140) Bump cloud version to 0.16.0 (RooCodeInc#7135) Release: v1.51.0 (RooCodeInc#7130) Add an API for resuming tasks by ID (RooCodeInc#7122) Add support for task page event population (RooCodeInc#7117) fix: add type check before calling .match() on diffItem.content (RooCodeInc#6905) (RooCodeInc#6906) Fix: Enable save button for provider dropdown and checkbox changes (RooCodeInc#7113) fix: Use cline.cwd as primary source for workspace path in codebaseSearchTool (RooCodeInc#6902) Hotfix multiple folder workspace checkpoint (RooCodeInc#6903) fix: prevent XML entity decoding in diff tools (RooCodeInc#7107) (RooCodeInc#7108) Refactor task execution system: improve call stack management (RooCodeInc#7035) Changeset version bump (RooCodeInc#7104) feat(web): fill missing SEO-related values (RooCodeInc#7096) Update contributors list (RooCodeInc#6883) Release v3.25.15 (RooCodeInc#7103) fix: add /evals page to sitemap generation (RooCodeInc#7102) feat: implement sitemap generation in TypeScript and remove XML file (RooCodeInc#6206) fix: reset condensing state when switching tasks (RooCodeInc#6922) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
The save button wasn't enabling when users changed provider settings like dropdowns and checkboxes. This was because
setApiConfigurationFieldwas treating all changes fromundefinedto a defined value as 'initial sync' and not marking the form as dirty.Solution
Added an optional
isUserActionparameter (defaults totrue) tosetApiConfigurationFieldto distinguish between:falseChanges Made
setApiConfigurationFieldinSettingsView.tsxto accept optionalisUserActionparameterisUserAction: false:ModelPicker.tsx- when auto-setting initial modelThinkingBudget.tsx- when auto-adjusting reasoning effort and thinking tokensApiOptions.tsx- when setting default models during provider switchingHuggingFace.tsx- when auto-setting default providerUnbound.tsx- when auto-selecting first available modelWhat This Fixes
✅ Checkboxes with default values (like
openRouterUseMiddleOutTransform ?? true) now enable the save button when clicked✅ Provider dropdown changes now enable the save button
✅ Any user interaction with settings properly marks the form as dirty
✅ Automatic initialization (like setting default models) won't falsely enable the save button
Testing
Fixes #[issue-number]
Important
Adds
isUserActionparameter tosetApiConfigurationFieldto enable save button for user changes in provider settings.isUserActionparameter tosetApiConfigurationFieldinSettingsView.tsxto differentiate user actions from automatic initializations.ModelPicker.tsx,ThinkingBudget.tsx,ApiOptions.tsx,HuggingFace.tsx, andUnbound.tsxto passisUserAction: falsefor automatic initializations.This description was created by
for 55f7792. You can customize this summary. It will automatically update as commits are pushed.